Skip to content

Moved imports into functions that use them #9722

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Moved imports into functions that use them #9722

wants to merge 1 commit into from

Conversation

cgrin
Copy link
Contributor

@cgrin cgrin commented Mar 25, 2015

closes #9713
closes #8327

Changes to be committed:
modified: pandas/io/gbq.py

Moved google api imports into functions that call them, removed unused imports. This fixes #9713

@cgrin
Copy link
Contributor Author

cgrin commented Mar 25, 2015

cc @jreback @jacobschaer

I've tested this change and successfully pulled data from and pushed to GBQ with it.

@jreback jreback added this to the 0.16.1 milestone Mar 25, 2015
jreback
jreback reviewed Mar 25, 2015
View reviewed changes
pandas/io/gbq.py
_GOOGLE_FLAGS_INSTALLED = True

_GOOGLE_FLAGS_VERSION = pkg_resources.get_distribution('python-gflags').version
if compat.PY3:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs to be moved into a function as this will raise on import of pandas when under py3.

@cgrin
Copy link
Contributor Author

cgrin commented Mar 25, 2015

@jreback good point. I've moved the Python3 check into the function that checks for compatibility with the google API

@jreback
Copy link
Contributor

jreback commented Apr 28, 2015

  • can you rebase
  • pls add a release note to 0.16.1

@jreback
Copy link
Contributor

jreback commented Apr 28, 2015

cc @jacobschaer

can you give a test out pls.

@cgrin
Copy link
Contributor Author

cgrin commented Apr 28, 2015

Rebase done and release note added.

@cgrin cgrin mentioned this pull request Apr 29, 2015
Closed
@jreback
Copy link
Contributor

jreback commented Apr 29, 2015

@cgrin looks like you merged in something else.

see instruction on git-fu here

@cgrin
Copy link
Contributor Author

cgrin commented Apr 29, 2015

Alright, that should take care of that. Not sure what happened yesterday. This closes #9713 and #8327

@jreback
Copy link
Contributor

jreback commented Apr 29, 2015

@cgrin the _test_imports is still called somewhere (and is failing)

@cgrin
Copy link
Contributor Author

cgrin commented Apr 29, 2015

@jreback Looks like it's called in the test suite. Since the _test_imports function itself gone due to the imports happening on demand, should we remove this test? It's the test_requirements function (lines 32-36 of pandas/io/tests/test_gbq.py) raising the error, which is in turn called by each class in the file.

@cgrin
Copy link
Contributor Author

cgrin commented Apr 29, 2015

We could also bring that function into test_gbq.py, removing the imports that are no longer necessary.

@jreback
Copy link
Contributor

jreback commented Apr 29, 2015

you can change the tests. The idea is to have a series of tests:

  • that will allow importing of gbq and not call anything (e.g. to test that if the deps are not installed simply importing will not fail)
  • use to/read_gbq with deps installed (which IIRC the ci does), but since we don't have credentials, they skip gracefully (e.g. the expected expections happen)
  • 'real' testing, currently done offline only (as credentials are necessary) that actually tests functionaility

@cgrin
Copy link
Contributor Author

cgrin commented Apr 29, 2015

Thanks. I opted to move this function into the test suite and remove the no longer needed imports. (Also rebased again as there has been some movement on the upstream repo). Hopefully this gets us where we need to be!

@cgrin
Copy link
Contributor Author

cgrin commented Apr 30, 2015

Looks like I missed a couple imports there. Apologies, I'm having issues getting the tests to run at all locally. 686ee7e should fix the compatibility tests.

@jreback
Copy link
Contributor

jreback commented Apr 30, 2015

@cgrin ok, looks good. if you could squash to a single commit would be gr8.

anything beyond the release note for #8327. e.g. would a user seeing this need to do anything?
this is prob not back-compat, but 0.16.0 is broken anyhow.....

Updates to Google BigQuery connector (#9713, #8327)
ea66ef1 
commit 3d6fdc8751134f6e5bac700519358cf9a700aba1
Author: Chris Grinolds <[email protected]>
Date:   Wed Apr 29 21:43:10 2015 -0700

    Added missing imports that caused tests to fail

commit 6c345d8dbc0c36bd4369dd220eceb495ab5ff2c6
Author: Chris Grinolds <[email protected]>
Date:   Wed Apr 29 16:23:01 2015 -0700

    Updated test suite to handle changes to gbq.py

commit f43e65f924d07f88bbbe308b9ea396b1f2a720f9
Author: Chris Grinolds <[email protected]>
Date:   Wed Apr 29 08:46:00 2015 -0700

    Updated BigQuery connector to no longer use deprecated ```oauth2client.tools.run() (#8327)

commit ea92e200ff341a3025b06868a565b1eea506c4c2
Author: Chris Grinolds <[email protected]>
Date:   Wed Apr 29 08:43:43 2015 -0700

    Import BigQuery dependencies on a per-method basis (#9713)
@cgrin
Copy link
Contributor Author

cgrin commented Apr 30, 2015

Rebased one more time and squashed everything.

Regarding #8327, the run function has been deprecated since at least June 2013, so it's very unlikely that anyone would need to take any action. google-api-python-client already has a version requirement of 1.2.0, which was released in July 2013 and includes the run_flow function, so this is more of a cleanup to get rid of the warning on auth than anything else.

@jreback
Copy link
Contributor

jreback commented May 1, 2015

merged via 2cf4132

thanks!

@jreback jreback closed this May 1, 2015
@jreback jreback mentioned this pull request May 1, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.16.1
Development

Successfully merging this pull request may close these issues.

On demand imports failing in pandas.io.gbq GBQ: Handle deprecation of oauth2client.tools.run()
2 participants
@cgrin @jreback